Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dex): solve token left in dex contract issue #8

Merged
merged 11 commits into from
Sep 13, 2023

Conversation

ryanycw
Copy link
Collaborator

@ryanycw ryanycw commented Sep 7, 2023

Summary

In this pull request we aim to solve the issue that sell token might left in dex contract.

Details

The main changes in this pull request is:

  • Changing paths storage from memory to calldata
  • Adding and testing tokenWithdraw feature in BasicDex which is inherited by all dexes.
  • Adding and testing afterSwap modifier in BasicDex which is inherited by all dexes.

@codecov-commenter
Copy link

Codecov Report

Merging #8 (7cdaaae) into build_pancake_dex (46c0279) will decrease coverage by 0.20%.
The diff coverage is 36.36%.

@@                  Coverage Diff                  @@
##           build_pancake_dex       #8      +/-   ##
=====================================================
- Coverage              36.36%   36.17%   -0.20%     
=====================================================
  Files                     14       16       +2     
  Lines                    319      329      +10     
  Branches                  33       34       +1     
=====================================================
+ Hits                     116      119       +3     
- Misses                   194      200       +6     
- Partials                   9       10       +1     
Files Changed Coverage Δ
src/core/dexes/BalancerDex.sol 95.83% <0.00%> (-4.17%) ⬇️
src/core/dexes/BancorV2Dex.sol 62.50% <0.00%> (-2.72%) ⬇️
src/core/dexes/CurveDex.sol 78.94% <0.00%> (-4.39%) ⬇️
src/core/dexes/PancakeV3Dex.sol 92.85% <0.00%> (-7.15%) ⬇️
src/core/dexes/UniBasedDex.sol 71.42% <0.00%> (-11.91%) ⬇️
src/core/dexes/UniV3Dex.sol 0.00% <0.00%> (ø)
test/AdvancedFixture.sol 0.00% <ø> (ø)
src/core/dexes/BasicDex.sol 50.00% <50.00%> (ø)
src/core/UniversalLiquidatorRegistry.sol 84.21% <100.00%> (ø)
test/mock/MockDex.sol 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

import "../../libraries/Errors.sol";

abstract contract BasicDex {
modifier afterSwapCheck(address _sellToken, address _buyToken) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the modifier being used anywhere. Is that correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thank you for the catch. I've added them to the actual DEX. I was only testing it in the mockDex.

@CryptJS13 CryptJS13 merged commit 45d16ba into build_pancake_dex Sep 13, 2023
2 checks passed
ryanycw added a commit that referenced this pull request Sep 20, 2023
* chore: change test helper file name

* build: implement basicDex with tokenWithdraw feature

* test: mock basic dex

* fix: add token withdraw to all dexes

* chore: adjust input type to calldata

* build: add invalid balance error

* test: add afterswap test

* refactor: add token check modifier to doSwap

* chore: consistent between optimizer

* fix: solve stack too deep issue

* fix: disable optimizer in test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants